Bring support for rendering from R file #7696
Conversation
This goes for now through knitr::spin()
src/execute/rmd.ts
Outdated
| if (ext == ".r") { | ||
| const text = Deno.readTextFileSync(file); | ||
| // Consider a .R script that can be spinned if first line starts with a special `#'` comment | ||
| return /^\s*#'/.test(text); |
There was a problem hiding this comment.
Is there something more we could look for? (as I think this would trigger for R code that uses any type of roxygen comment e.g. a package source file or plumber file)
There was a problem hiding this comment.
Oh indeed. I did not thought package files would be found while rendering with quarto but probably better to be safe here.
There is no specific spin syntax except #+ for chunk options but I believe we prefer to use yaml.
Probably best is requiring a yaml header in the file. First like would need to be #´ ---. This would require a user to add one.
Otherwise we could require a specific fields for quarto. # /* is a special comment that spin will ignore and won't affect output.
So something like
# /* quarto */
but this is quite new and different syntax.
Maybe requiring yaml header is the simplest here and not too much to ask.
|
Yes, I think the YAML header is the thing to do
…On Fri, Nov 24, 2023 at 3:33 PM Christophe Dervieux < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/execute/rmd.ts
<#7696 (comment)>
:
> @@ -353,3 +371,35 @@ function resolveInlineExecute(code: string) {
(expr) => `${"`"}r .QuartoInlineRender(${expr})${"`"}`,
)(code);
}
+
+export function isKnitrSpinScript(file: string) {
+ const ext = extname(file).toLowerCase();
+ if (ext == ".r") {
+ const text = Deno.readTextFileSync(file);
+ // Consider a .R script that can be spinned if first line starts with a special `#'` comment
+ return /^\s*#'/.test(text);
Oh indeed. I did not thought package files would be found while rendering
with quarto but probably better to be safe here.
There is no specific spin syntax except #+ for chunk options but I
believe we prefer to use yaml.
Probably best is requiring a yaml header in the file. First like would
need to be #´ ---. This would require a user to add one.
Otherwise we could require a specific fields for quarto. # /* is a
special comment that spin will ignore and won't affect output.
So something like
# /* quarto */
but this is quite new and different syntax.
Maybe requiring yaml header is the simplest here and not too much to ask.
—
Reply to this email directly, view it on GitHub
<#7696 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAZPRY23DJ37CGPAOG5ARDYGEAARAVCNFSM6AAAAAA7ZP2S4OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBYGQYDSNBQG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Ok. I agree too. That is better. About the two passes we do for the markdown content import which leads to two runs of spin(), do you have insights ? I am thinking of caching first spin results in temp folder but not sure this is robust. It feels unneeded but I may be missing something. |
|
I wouldn't try to untangle why that is called twice or do any caching. I actually tried to do this already for Jupyter and I broke something I didn't understand related to includes. I think the better solution is to just write a typescript version that is cheaper (don't need to do this for this PR but perhaps a follow-up PR?) |
Let's try to fix that before 1.4 is in - I'll open an issue. |
|
So exciting y'all. Made a little demo video here: https://x.com/kylefbutts/status/1729145164881674539?s=20 |
|
@cderv We should also make sure that RStudio knows to make "Preview" available for R scripts. |
This closes #6660
It does the same logic as for Jupyter in #6700
Though proposal here is to use
knitr::spin()directly, which has support for.qmdsince v1.44.There is question about performance, but we could easily improve by implementing the
knitr::spin()logic internally in TypeScript. I'll suggest we wait for feedback on usage with calling R directly.It works ok though little thing I noticed : It seems we run the
asMappedString()from a input file at two placesInside
target()in our callquarto-cli/src/execute/engine.ts
Line 214 in 1a8c1af
where we run
quarto-cli/src/execute/rmd.ts
Line 84 in 74391fc
and then a second time when rendering project as we do run
quarto-cli/src/project/project-config.ts
Line 97 in 1a8c1af
to find resources and this calls
quarto-cli/src/project/project-config.ts
Line 110 in 74391fc
which needs to get markdown from input file too.
I am not sure why we need to run all this twice instead of using a stored version from the first run. But I am probably missing something.
Should we try caching the results of first spin ? Or that could cause issue ?
On other topic, I'll add a documentation file for the website. Let's note that
#+is not specifically needed and YAML syntax can directly be used this way.This makes it quite coherent with percent script syntax I believe. I'll probably document only this way on Quarto Web, but the
#+still works.Thanks